feat: Added support for custom maxTransactionFee and gas to createContract/callContract#249
Open
manishdait wants to merge 7 commits into
Open
feat: Added support for custom maxTransactionFee and gas to createContract/callContract#249manishdait wants to merge 7 commits into
maxTransactionFee and gas to createContract/callContract#249manishdait wants to merge 7 commits into
Conversation
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Contributor
|
Hey @manishdait 👋 thanks for the PR! This comment updates automatically as you push changes -- think of it as your PR's live scoreboard! PR Checks✅ DCO Sign-off -- All commits have valid sign-offs. Nice work! ✅ GPG Signature -- All commits have verified GPG signatures. Locked and loaded! ✅ Merge Conflicts -- No merge conflicts detected. Smooth sailing! ✅ Issue Link -- Linked to #242 (assigned to you). 🎉 All checks passed! Your PR is ready for review. Great job! |
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Request contracts and validation hiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/*Request.java, hiero-enterprise-base/src/test/java/org/hiero/base/test/ProtocolLayerDataCreationTests.java |
Creation and call requests now carry explicit gas and transaction fees, validate gas against MAX_GAS_LIMIT, and expose updated factories. |
Client overload delegation and implementation hiero-enterprise-base/src/main/java/org/hiero/base/SmartContractClient.java, hiero-enterprise-base/src/main/java/org/hiero/base/implementation/SmartContractClientImpl.java, hiero-enterprise-base/src/test/java/org/hiero/base/test/SmartContractClientImplTest.java |
Default interface overloads delegate fee and gas values, while implementations validate inputs and populate contract requests. |
Protocol gas propagation hiero-enterprise-base/src/main/java/org/hiero/base/implementation/ProtocolLayerClientImpl.java |
Contract-create and contract-call transactions use gas from their requests. |
Integration coverage hiero-enterprise-microprofile/src/test/java/org/hiero/microprofile/test/SmartContractClientTest.java, hiero-enterprise-spring/src/test/java/org/hiero/spring/test/* |
Integration tests cover contract creation, calls, parameters, results, explicit request values, and corrected resource lookups. |
Estimated code review effort: 4 (Complex) | ~45 minutes
Sequence Diagram(s)
sequenceDiagram
participant Caller
participant SmartContractClient
participant SmartContractClientImpl
participant ProtocolLayerClientImpl
participant HederaTransaction
Caller->>SmartContractClient: createContract or callContractFunction
SmartContractClient->>SmartContractClientImpl: delegate fee and gas
SmartContractClientImpl->>ProtocolLayerClientImpl: execute populated request
ProtocolLayerClientImpl->>HederaTransaction: set gas from request.gas()
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly summarizes the main change: custom max fee and gas support for contract create/call. |
| Description check | ✅ Passed | The description matches the changeset and describes the fee/gas overloads and test updates. |
| Linked Issues check | ✅ Passed | The PR addresses #242 by raising the default create-contract fee and adding custom fee/gas support. |
| Out of Scope Changes check | ✅ Passed | The additional protocol, test, and resource-reference changes all support the stated contract fee/gas work. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c731f637-dcb7-4242-9e43-1cf49f5430a2
⛔ Files ignored due to path filters (5)
hiero-enterprise-microprofile/src/test/resources/datatypes.binis excluded by!**/*.binhiero-enterprise-microprofile/src/test/resources/large_contract.binis excluded by!**/*.binhiero-enterprise-microprofile/src/test/resources/small_contract.binis excluded by!**/*.binhiero-enterprise-microprofile/src/test/resources/string_param_constructor_contract.binis excluded by!**/*.binhiero-enterprise-microprofile/src/test/resources/uint_getter_setter_contract.binis excluded by!**/*.bin
📒 Files selected for processing (11)
hiero-enterprise-base/src/main/java/org/hiero/base/SmartContractClient.javahiero-enterprise-base/src/main/java/org/hiero/base/implementation/ProtocolLayerClientImpl.javahiero-enterprise-base/src/main/java/org/hiero/base/implementation/SmartContractClientImpl.javahiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCallRequest.javahiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCreateRequest.javahiero-enterprise-base/src/test/java/org/hiero/base/test/ProtocolLayerDataCreationTests.javahiero-enterprise-base/src/test/java/org/hiero/base/test/SmartContractClientImplTest.javahiero-enterprise-microprofile/src/test/java/org/hiero/microprofile/test/SmartContractClientTest.javahiero-enterprise-spring/src/test/java/org/hiero/spring/test/ProtocolLayerClientTests.javahiero-enterprise-spring/src/test/java/org/hiero/spring/test/SmartContractClientTest.javahiero-enterprise-spring/src/test/java/org/hiero/spring/test/SmartContractDatatypesTests.java
| private static final Logger log = LoggerFactory.getLogger(ProtocolLayerClientImpl.class); | ||
|
|
||
| public static final int DEFAULT_GAS = 5_000_000; | ||
| public static final int MAX_GAS_LIMIT = 15_000_000; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Place shared gas policy in a neutral API/domain type. Defining it on ProtocolLayerClientImpl reverses dependencies throughout the request and public API layers.
hiero-enterprise-base/src/main/java/org/hiero/base/implementation/ProtocolLayerClientImpl.java#L123-L123: moveMAX_GAS_LIMITandDEFAULT_GASto the neutral type.hiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCallRequest.java#L3-L4: import the limit from that neutral type.hiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCreateRequest.java#L3-L4: import the limit from that neutral type.hiero-enterprise-base/src/main/java/org/hiero/base/SmartContractClient.java#L3-L5: import the default gas without depending on an implementation.hiero-enterprise-base/src/main/java/org/hiero/base/implementation/SmartContractClientImpl.java#L3-L3: consume the same neutral policy.
📍 Affects 5 files
hiero-enterprise-base/src/main/java/org/hiero/base/implementation/ProtocolLayerClientImpl.java#L123-L123(this comment)hiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCallRequest.java#L3-L4hiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCreateRequest.java#L3-L4hiero-enterprise-base/src/main/java/org/hiero/base/SmartContractClient.java#L3-L5hiero-enterprise-base/src/main/java/org/hiero/base/implementation/SmartContractClientImpl.java#L3-L3
Comment on lines
86
to
90
| try { | ||
| final FileId fileId = fileClient.createFile(contents); | ||
| final ContractId contract = createContract(fileId, constructorParams); | ||
| final ContractId contract = createContract(fileId, maxTransactionFee, gas, constructorParams); | ||
| fileClient.deleteFile(fileId); | ||
| return contract; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Always delete the temporary bytecode file after creation attempts.
If delegated gas validation or contract creation fails, Line 89 is skipped, leaving a paid Hedera file orphaned. Validate before upload where possible and delete the file in finally; the Path overload inherits this leak.
Proposed cleanup
+ FileId fileId = null;
try {
- final FileId fileId = fileClient.createFile(contents);
- final ContractId contract = createContract(fileId, maxTransactionFee, gas, constructorParams);
- fileClient.deleteFile(fileId);
- return contract;
+ fileId = fileClient.createFile(contents);
+ return createContract(fileId, maxTransactionFee, gas, constructorParams);
} catch (Exception e) {
throw new HieroException("Failed to create contract out of byte array", e);
+ } finally {
+ if (fileId != null) {
+ fileClient.deleteFile(fileId);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| final FileId fileId = fileClient.createFile(contents); | |
| final ContractId contract = createContract(fileId, constructorParams); | |
| final ContractId contract = createContract(fileId, maxTransactionFee, gas, constructorParams); | |
| fileClient.deleteFile(fileId); | |
| return contract; | |
| FileId fileId = null; | |
| try { | |
| fileId = fileClient.createFile(contents); | |
| return createContract(fileId, maxTransactionFee, gas, constructorParams); | |
| } catch (Exception e) { | |
| throw new HieroException("Failed to create contract out of byte array", e); | |
| } finally { | |
| if (fileId != null) { | |
| fileClient.deleteFile(fileId); | |
| } | |
| } |
Comment on lines
42
to
88
| @NonNull | ||
| public static ContractCreateRequest of( | ||
| @NonNull String fileId, @Nullable ContractParam<?>... constructorParams) { | ||
| @NonNull String fileId, | ||
| @NonNull Hbar maxTransactionFee, | ||
| int gas, | ||
| @Nullable ContractParam<?>... constructorParams) { | ||
| Objects.requireNonNull(fileId, "fileId must not be null"); | ||
| return of(FileId.fromString(fileId), constructorParams); | ||
| Objects.requireNonNull(maxTransactionFee, "maxTransactionFee must not be null"); | ||
| return of(FileId.fromString(fileId), maxTransactionFee, gas, constructorParams); | ||
| } | ||
|
|
||
| @NonNull | ||
| public static ContractCreateRequest of( | ||
| @NonNull FileId fileId, @Nullable ContractParam<?>... constructorParams) { | ||
| @NonNull FileId fileId, | ||
| @NonNull Hbar maxTransactionFee, | ||
| int gas, | ||
| @Nullable ContractParam<?>... constructorParams) { | ||
| if (constructorParams == null) { | ||
| return of(fileId, List.of()); | ||
| return of(fileId, maxTransactionFee, gas, List.of()); | ||
| } else { | ||
| return of(fileId, List.of(constructorParams)); | ||
| return of(fileId, maxTransactionFee, gas, List.of(constructorParams)); | ||
| } | ||
| } | ||
|
|
||
| @NonNull | ||
| public static ContractCreateRequest of( | ||
| @NonNull String fileId, @NonNull List<ContractParam<?>> constructorParams) { | ||
| @NonNull String fileId, | ||
| @NonNull Hbar maxTransactionFee, | ||
| int gas, | ||
| @NonNull List<ContractParam<?>> constructorParams) { | ||
| Objects.requireNonNull(fileId, "fileId must not be null"); | ||
| return of(FileId.fromString(fileId), constructorParams); | ||
| Objects.requireNonNull(maxTransactionFee, "maxTransactionFee must not be null"); | ||
| return of(FileId.fromString(fileId), maxTransactionFee, gas, constructorParams); | ||
| } | ||
|
|
||
| @NonNull | ||
| public static ContractCreateRequest of( | ||
| @NonNull FileId fileId, @NonNull List<ContractParam<?>> constructorParams) { | ||
| @NonNull FileId fileId, | ||
| @NonNull Hbar maxTransactionFee, | ||
| int gas, | ||
| @NonNull List<ContractParam<?>> constructorParams) { | ||
| return new ContractCreateRequest( | ||
| DEFAULT_MAX_TRANSACTION_FEE, | ||
| maxTransactionFee, | ||
| DEFAULT_TRANSACTION_VALID_DURATION, | ||
| fileId, | ||
| gas, | ||
| List.copyOf(constructorParams)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add fee/gas factories without removing the existing public signatures. Otherwise this feature introduces an avoidable source-compatibility break.
hiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCreateRequest.java#L42-L88: restore old factories and delegate with the 16-HBAR create fee and default gas.hiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCallRequest.java#L46-L98: restore old factories and delegate with the standard transaction fee and default gas.
📍 Affects 2 files
hiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCreateRequest.java#L42-L88(this comment)hiero-enterprise-base/src/main/java/org/hiero/base/protocol/data/ContractCallRequest.java#L46-L98
Contributor
|
@manishdait is this ready for review? |
Contributor
Author
|
Yes :) |
aceppaluni
suggested changes
Jul 20, 2026
| Objects.requireNonNull(maxTransactionFee, "maxTransactionFee must not be null"); | ||
|
|
||
| try { | ||
| final FileId fileId = fileClient.createFile(contents); |
Contributor
There was a problem hiding this comment.
If createContract(...) throws, is deleteFile() ever called?
Should it be
final FileId fileId = fileClient.createFile(contents);
try {
return createContract(fileId, maxTransactionFee, gas, constructorParams);
} finally {
fileClient.deleteFile(fileId);
}
Contributor
Author
There was a problem hiding this comment.
that is to matche the existing flow we have, but happy to change the flow in this PR it needed :)
| final ContractId contract = createContract(fileId, maxTransactionFee, gas, constructorParams); | ||
| fileClient.deleteFile(fileId); | ||
| return contract; | ||
| } catch (Exception e) { |
Contributor
There was a problem hiding this comment.
Would it make sense to catch more specific exceptions (e.g. IOException) here? Catching Exception also wraps programming errors like IllegalArgumentException, which may make debugging harder.
Contributor
Author
There was a problem hiding this comment.
same with this one too
aceppaluni
added
the
status: needs developer revision
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
This PR adds support for set custom
maxTransactionFeeandgasforcreateContract()andcallContractFunction()Changes Made
createContract()andcallContractFunction()to set customgasandmaxTransactionFee.maxTransactionFeeforcreateContract()from 10 HBAR to 16 HBAR.Related issue(s):
Fixes #242
Notes for reviewer:
The default
maxTransactionFeeforcreateContract()increased to 16 HBAR, which equivalent to the current smart contract service fee docs1$approx15.8 HBARSee Here.Overload method to support the
maxTransactionFeeandgasso that if works with the large contracts.Checklist